[HUDI-875] Introduce a new pom module named hudi-common-sync#1716
[HUDI-875] Introduce a new pom module named hudi-common-sync#1716lw309637554 wants to merge 1 commit intoapache:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1716 +/- ##
=============================================
- Coverage 72.25% 18.14% -54.11%
- Complexity 294 859 +565
=============================================
Files 374 352 -22
Lines 16371 15411 -960
Branches 1654 1525 -129
=============================================
- Hits 11829 2797 -9032
- Misses 3806 12256 +8450
+ Partials 736 358 -378 Continue to review full report at Codecov.
|
|
@vinothchandar What do you think about the PR, introducing a new module. |
|
Good with the module.. Few thoughts/suggestions
|
hudi-sync-common/pom.xml
Outdated
There was a problem hiding this comment.
is this pom same as the current hudi-hive-sync pom?
There was a problem hiding this comment.
Good with the module.. Few thoughts/suggestions
- Can we nest this under hudi-sync as
hudi-sync-common,hudi-sync-hiveand you can then introduce the aliyun metastore ashudi-sync-aliyun?- We need to just ensure there are no upgrades/backwards incompatible changes for existing users .
thanks ,
- i think "we nest this under hudi-sync as
hudi-sync-common" is a good idea,i will modify like this - "there are no upgrades/backwards incompatible changes for existing users ." i will add more tests to guarantee
There was a problem hiding this comment.
is this pom same as the current hudi-hive-sync pom?
not exactly the same,i will minimize it
lw309637554
force-pushed
the
HUDI-875
branch
2 times, most recently
from
ef3c13f to
41a10f6
Compare
|
@vinothchandar What do you think about the updated PR |
vinothchandar
left a comment
vinothchandar
left a comment
There was a problem hiding this comment.
is the plan to eventually also move hudi-hive-sync under hudi-sync?
|
@umehrot2 @zhedoubushishi please weigh in with your thoughts as well |
yes，when /hudi-sync/hudi-sync-common is ready, will move hudi-hive-sync under hudi-sync |
|
Sorry for the delay. Was spending time on another large review. Can we make those changes in this pr itself? Would love to see end-end functionality working. IIRC your ultimate goal is to plug in alliyun metastore? |
ok , then i add the changes in this pr. aliyun metastore can plug is valuable for users.Now users already use kafka -> spark -> hudi + aliyun metastore -> presto + spark program. |
Thanks for letting me know.. Thats great! I am a fan of doing the refactor with a precise goal in mind.. so please feel free to reopen if you feel we need to restructure for something else.. AWS Glue also works with the current abstractions.. |
4 participants
Tips
What is the purpose of the pull request
Introduce a new pom module named hudi-common-sync, before start abstract common meta sync module support multiple meta service. Design is in https://cwiki.apache.org/confluence/display/HUDI/RFC+-+17+Abstract+common+meta+sync+module+support+multiple+meta+service
Brief change log
Introduce a new pom module named hudi-common-sync, before start abstract common meta sync module support multiple meta service.
Verify this pull request
(Please pick either of the following options)
This pull request is a trivial rework / code cleanup without any test coverage.
Committer checklist
Has a corresponding JIRA in PR title & commit
Commit message is descriptive of the change
CI is green
Necessary doc changes done or have another open PR
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.